Skip to content

Conversation

@casabre
Copy link
Contributor

@casabre casabre commented Jul 20, 2025

Description

Adding Sequence support for audience in JWT Token.

Closes

#4240

@casabre casabre requested review from a team as code owners July 20, 2025 10:52
@casabre casabre changed the title Audience should support sequence too fix: audience should support sequence too Jul 20, 2025
@codecov
Copy link

codecov bot commented Jul 20, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.93%. Comparing base (b105cff) to head (4d023c6).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4241   +/-   ##
=======================================
  Coverage   97.93%   97.93%           
=======================================
  Files         319      319           
  Lines       15583    15584    +1     
  Branches     1726     1726           
=======================================
+ Hits        15261    15262    +1     
  Misses        184      184           
  Partials      138      138           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@igorcoding
Copy link

igorcoding commented Sep 17, 2025

Hi! Is there any chance that this would be merged soon? Our team is currently experiencing an issue with the aud being an array. Or Maybe you know of any workaround to mitigate this issue without patching source code?

@Harshal6927
Copy link
Member

Hello @casabre , thanks for the PR! Could you please add a test case so we can go ahead and merge it?

@casabre
Copy link
Contributor Author

casabre commented Sep 18, 2025

@Harshal6927 @provinzkraut I added an own test for audience validation. Hope that works for you.

@casabre
Copy link
Contributor Author

casabre commented Sep 18, 2025

@provinzkraut I didn't change the imports but now pytest is complaining about a missing Sequence import during runtime. Actually, I don't know if this is related to a pytest version change and if I should fix the imports or leave it as it is...

@casabre
Copy link
Contributor Author

casabre commented Sep 19, 2025

@provinzkraut In the end, a missing import makes the pytests fail

FAILED tests/unit/test_security/test_jwt/test_auth.py::test_jwt_auth - NameError: name 'Sequence' is not defined

e.g. at https://github.com/litestar-org/litestar/actions/runs/17854788852/job/50771227842?pr=4241#step:10:9032

@provinzkraut
Copy link
Member

@provinzkraut I didn't change the imports but now pytest is complaining about a missing Sequence import during runtime. Actually, I don't know if this is related to a pytest version change and if I should fix the imports or leave it as it is...

Well it's because you are using Sequence but don't import it anywhere 😉

@casabre
Copy link
Contributor Author

casabre commented Sep 19, 2025

@provinzkraut I didn't change the imports but now pytest is complaining about a missing Sequence import during runtime. Actually, I don't know if this is related to a pytest version change and if I should fix the imports or leave it as it is...

Well it's because you are using Sequence but don't import it anywhere 😉

Yes, you right 😅. But I was relying on the correctness of previous logic --> because Sequence was used before my change I thus, I thought, it is covered. E.g here

issuer: str | Sequence[str] | None = None,

It's not a problem for me to pull the collections import out of the type checking guard, but I am really wondering how it was working before 🤔

@provinzkraut
Copy link
Member

It's not a problem for me to pull the collections import out of the type checking guard, but I am really wondering how it was working before 🤔

The function signature is not validated during runtime. The dataclass itself is.

@casabre
Copy link
Contributor Author

casabre commented Sep 19, 2025

It's not a problem for me to pull the collections import out of the type checking guard, but I am really wondering how it was working before 🤔

The function signature is not validated during runtime. The dataclass itself is.

@provinzkraut Yes, I understand that but why was it failing now but before not, when Sequence was accessed too. Anyway... let't move ahead --> if you are satisfied with the change, can you just approve it 😊.

@casabre
Copy link
Contributor Author

casabre commented Oct 2, 2025

@provinzkraut what do you say to the changes? Can we move forward and merge?

@provinzkraut provinzkraut changed the title fix: audience should support sequence too fix(jwt): Relax typing to allow Sequence for Token.aud Oct 3, 2025
@provinzkraut provinzkraut enabled auto-merge (squash) October 3, 2025 11:45
@provinzkraut provinzkraut merged commit b97de01 into litestar-org:main Oct 3, 2025
28 checks passed
@github-actions
Copy link

github-actions bot commented Oct 3, 2025

Documentation preview will be available shortly at https://litestar-org.github.io/litestar-docs-preview/4241

@casabre casabre deleted the fix/audience-issue#4240 branch October 26, 2025 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants